fix(oauth): skip invalid redirect URIs in DCR instead of failing registration - #152
Closed
crimsonsunset wants to merge 2 commits into
Closed
fix(oauth): skip invalid redirect URIs in DCR instead of failing registration#152crimsonsunset wants to merge 2 commits into
crimsonsunset wants to merge 2 commits into
Conversation
…re registration Cursor and other MCP clients send a mix of valid (custom scheme + loopback) and invalid (https non-loopback) redirect URIs in a single DCR request. Previously, any invalid URI caused the entire registration to fail with `invalid_redirect_uri`, preventing affected clients from connecting at all. This change filters out invalid URIs and only fails when zero valid URIs remain. The behavior matches what other MCP-aware OAuth implementations have already adopted (e.g. supabase/auth#0fed91a, Python MCP SDK, TanStack MCP). Example: Cursor 3.4.20 sends: - cursor://anysphere.cursor-mcp/oauth/callback (valid, custom scheme) - https://www.cursor.com/agents/mcp/oauth/callback (invalid, https) - http://localhost:8787/callback (valid, loopback) Before: DCR returns 400, Cursor cannot register or connect. After: invalid URI is skipped with a warn log, valid URIs are stored, Cursor registers successfully. Signed-off-by: crimsonsunset <jsangio1@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR changes OAuth DCR redirect URI handling so mixed valid/invalid redirect URI registration requests can succeed instead of failing the entire registration.
Changes:
- Updates DCR redirect URI validation to allow registration when at least one URI is valid.
- Logs invalid redirect URIs as skipped instead of immediately returning an error.
- Adds unit tests for mixed-validity and all-invalid redirect URI scenarios.
Comments suppressed due to low confidence (2)
crates/mcpmux-gateway/src/oauth/dcr.rs:214
- This change only skips invalid URIs during validation; it does not remove them from the registration.
process_dcr_requeststill merges/saves/returns the originalrequest.redirect_uris, and the authorization endpoint later accepts any stored URI via an exactcontainscheck, so a non-loopback HTTPS URI in a mixed request becomes a registered redirect URI despite the log saying it was skipped. The validation needs to return/use the filtered list (for both create and update paths) or invalid redirect URIs remain allowed.
if !is_loopback && !is_custom_scheme {
// Skip invalid URIs (e.g. https://www.cursor.com/agents/mcp/oauth/callback)
// rather than rejecting the entire registration — clients like Cursor send a
// mix of valid and invalid URIs and only ever use the valid ones in practice.
warn!(
"[DCR] Skipping invalid redirect_uri: {} (must be loopback or custom scheme)",
uri
);
continue;
crates/mcpmux-gateway/src/oauth/dcr.rs:455
- This test only checks that mixed URIs no longer fail validation, but it does not assert the new "invalid URIs are skipped" behavior through registration storage/response. Because the production path still saves and returns the original URI list, this test passes while the invalid URI remains registered; add coverage for
process_dcr_request(or for a filtering helper) that verifies invalid entries are absent from the stored client and DCR response.
let uris = vec![
"cursor://anysphere.cursor-mcp/oauth/callback".to_string(),
"https://www.cursor.com/agents/mcp/oauth/callback".to_string(),
"http://localhost:8787/callback".to_string(),
];
assert!(validate_redirect_uris(&uris).is_ok());
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
its-mash
previously approved these changes
May 16, 2026
Fixes clippy::unnecessary_sort_by lint causing CI failure. Signed-off-by: Joe Sangiorgio <jsangio1@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
When will this be merged in? its blocking claude code for me |
Member
|
merged by #158 . Thanks @crimsonsunset |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
validate_redirect_urisincrates/mcpmux-gateway/src/oauth/dcr.rshard-fails the entire DCR registration if any redirect URI in the request fails validation. This blocks Cursor (and likely other MCP clients) from connecting because Cursor 3.4.20 sends three redirect URIs in a single DCR request — two valid, one invalid:Real-world log output from McpMux v0.3.0:
Cursor cannot complete OAuth, so it cannot connect to the McpMux gateway at all.
Fix
Skip invalid URIs with a warn log instead of returning an error. Only fail the registration if zero valid URIs remain after filtering.
This is what other MCP-aware OAuth implementations have already converged on:
cursor://anysphere.cursor-mcpto allowlistBehavior change
Tests
Added two new unit tests covering the mixed-validity and all-invalid cases:
test_mixed_valid_and_invalid_uris_pass— reproduces the exact Cursor scenariotest_all_invalid_uris_fail— ensures we don't accidentally accept clients with only invalid URIsAll 5
oauth::dcrtests pass locally:Manual verification
Built locally on macOS arm64, replaced the bundled binary in
/Applications/McpMux.app, and confirmed:[DCR] Successfully registered client: Cursor (mcp_36740f70))needsAuth→connectedNotes
-s)